Claude finished @jcastro-dotcms's task in 46s —— View job

Rollback Safety Analysis — ✅ Safe to Rollback

Analyzed the PR diff against every category in ROLLBACK_UNSAFE_CATEGORIES.md.

Changes are purely additive / UI-only:

• Feature-flag constant FEATURE_FLAG_REPORT_ISSUE_ENABLED (default false)
• Whitelist addition in ConfigurationResource (additive)
• Angular component flag gating + tests
• Report-issue HTML-error fallback handling
• New translation key (additive)

No matches for any unsafe category — no DB migrations, ES mapping changes, contentlet_as_json model bump, DROP/RENAME/PK changes, new field types, storage provider changes, procedure drops, NOT NULL columns, column type changes, push publishing schema changes, REST contract removals/renames, or OSGi interface changes.

N-1 after rollback simply reverts to the pre-PR behavior (toolbar item shown unconditionally). The new whitelisted config key being absent from /api/v1/configuration/config in N-1 is the original behavior — no client breakage.

Label AI: Safe To Rollback applied.
• Branch: fmontes/feature-request-flag

Thanks for the review @claude — addressed in the latest force-push:

1. combineLatest regression ✅ Fixed
Wrapped the flag observable in catchError(() => of(false)) so a failed /api/v1/configuration/config call degrades to "flag off" instead of taking the whole toolbar down with it.

2. Disabled-path test coverage ✅ Fixed
Added should hide the report issue menu item when the feature flag is disabled — overrides the DotPropertiesService mock to return of(false) and asserts the item is absent from vm.items. Now both default-production and enabled states are exercised.

3. shareReplay(1) to future-proof double-subscription ⏭️ Skipped
Only one vm$ | async subscriber today. Adding it preemptively for a hypothetical second subscriber felt like YAGNI, especially since the whole flag check is meant to be ripped out after launch.

4. Extract menu-item id constant ⏭️ Skipped
The duplication already existed in the store before this PR — the cleanup belongs in its own change rather than bundled here.

Claude finished @fmontes's task in 1m 40s —— View job

Verifying the latest changes

• Read the current state of the changed files on the branch
• Verify fix update with latest SVN #1 (catchError resilience) is in place
• Verify fix Test Branch and Commit #2 (disabled-path test) is in place
• Cross-check the third original review point about the REPORT_ISSUE_INCLUDE_USER_PII constant on ConfigurationResource
• Post review summary

Verification

Fix #1 — catchError resilience ✅ Confirmed at dot-toolbar-user.component.ts:51-57. The flag observable is now .pipe(catchError(() => of(false))) and the inline comment clearly states the intent ("Degrade to 'flag off' on fetch errors so the whole toolbar doesn't disappear.").

One minor thing worth surfacing: you went with catchError alone, not startWith(false) + catchError. That means errored requests are now handled, but a slow /api/v1/configuration/config will still gate the whole vm$ | async template (no avatar/menu until the call emits). In practice this is probably fine — the endpoint is fast on a healthy backend — but if you ever see toolbar flicker on cold starts, that's the knob to turn.

Fix #2 — Disabled-path test ✅ Confirmed at dot-toolbar-user.component.spec.ts:268-286. The new spec re-mocks getFeatureFlagWithDefault to of(false), rebuilds the component so the new value flows through combineLatest, and asserts the report-issue item is undefined in vm.items. The default-true setup at lines 70-75 keeps the enabled path covered. 👍

Skipped #3/#4 — YAGNI calls and scope hygiene both look fine to me. shareReplay(1) can wait until there's an actual second subscriber, and the menu-item id constant cleanup is genuinely pre-existing churn that's better in its own PR.

One copilot point that didn't get a response

The copilot reviewer's third comment about ConfigurationResource.java:75 still applies on the current HEAD — the whitelist uses the literal "REPORT_ISSUE_INCLUDE_USER_PII" even though REPORT_ISSUE_INCLUDE_USER_PII is declared as a constant one line above (L61) and is already used at L144 for the default-value lookup. It's a trivial swap to remove the duplication. Up to you whether to fold it into this PR or leave it — same scope argument as #4 applies. Fix this →

Net

The two fixes you committed look correct and the test coverage matches what was asked for. From my side this is good to go once the lint/test job comes back green.
• Branch: fmontes/feature-request-flag